fix: properly parse xml with no replacements by 2bndy5 · Pull Request #230 · cpp-linter/cpp-linter-rs

2bndy5 · 2025-12-11T16:44:08Z

Fixing XML parsing caused another problem to surface: Errors were ignored during capture of clang tools output. A side effect of that fix required some test changes about PR reviews (LGTM scenario).

Summary by CodeRabbit

Bug Fixes
- Graceful handling when no formatting replacements are returned; avoids unexpected failures.
- Lint review ignore rules separated for formatting vs. tidy, so each tool can ignore different file sets.
Tests
- Added tests for parsing inputs with no replacements.
- Tightened test assertions to fail fast on unexpected results.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-11T16:48:48Z

Walkthrough

Adds Default derive and serde default for FormatAdvice.replacements; uses FormatAdvice::default() in no-output code path; changes task-output handling to early-return on errors via unwraps; converts several test assertions to direct unwrap/unwrap_err; splits review test ignore patterns into separate format/tidy variables.

Changes

Cohort / File(s)	Summary
FormatAdvice and XML parsing `cpp-linter/src/clang_tools/clang_format.rs`	Add `Default` derive to `FormatAdvice`; add `#[serde(rename(deserialize = "replacement"), default)]` to `replacements`; replace manual no-output struct init with `FormatAdvice::default()`; add unit tests (`parse_xml_no_replacements`) validating deserialization when no `<replacement/>` entries exist.
Clang tools output handling `cpp-linter/src/clang_tools/mod.rs`	Replace conditional per-join handling with direct unwrap propagation (`output??` style), making failures early-return and moving logging to follow the unwrap path.
Test assert style updates `cpp-linter/src/run.rs`	Replace `.await` + `assert!(is_ok/is_err)` patterns with direct `.await.unwrap()` / `.await.unwrap_err()` in tests.
Review test ignore-splitting `cpp-linter/tests/reviews.rs`	Split shared ignore token into `format_ignore` and `tidy_ignore`; expand each differently when `force_lgtm` is true; pass them to `--ignore-format` / `--ignore-tidy` respectively.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Inspect clang_tools/mod.rs for behavioral changes around error propagation and logging order.
Verify FormatAdvice::default() and #[serde(..., default)] produce the intended empty-state (especially replacements).
Confirm tests switched to unwrap/unwrap_err still assert intended failure modes rather than panicking prematurely.
Check review ignore-splitting logic to ensure file-globs match expected sets when force_lgtm is enabled.

Possibly related PRs

refactor: switch to quick_xml library #101 — Changes touching XML parsing and replacement handling in clang_format.rs, closely related to the FormatAdvice deserialization/defaults adjustments.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding Default derive and serde default attribute to FormatAdvice to properly handle XML parsing when there are no replacements.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d8d252d and 96dff79.

📒 Files selected for processing (4)

cpp-linter/src/clang_tools/clang_format.rs (3 hunks)
cpp-linter/src/clang_tools/mod.rs (1 hunks)
cpp-linter/src/run.rs (3 hunks)
cpp-linter/tests/reviews.rs (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

When capturing clang tools output, any error encountered should cause fast-failure behavior. Also adjust run_main() tests so that erroneous output is more helpful..

2bndy5 · 2025-12-11T18:48:56Z

@coderabbitai help

2bndy5 · 2025-12-11T18:49:59Z

@coderabbitai summary

coderabbitai · 2025-12-11T18:50:06Z

✅ Actions performed

Summary regeneration triggered.

codecov · 2025-12-11T18:56:18Z

Codecov Report

❌ Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.90%. Comparing base (82913d6) to head (96dff79).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
cpp-linter/src/clang_tools/clang_format.rs	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #230      +/-   ##
==========================================
+ Coverage   96.79%   96.90%   +0.10%     
==========================================
  Files          14       14              
  Lines        3027     3034       +7     
==========================================
+ Hits         2930     2940      +10     
+ Misses         97       94       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fix: properly parse xml with no replacements

4f5f9e6

2bndy5 added the bug Something isn't working label Dec 11, 2025

This comment was marked as resolved.

Sign in to view

2bndy5 added 3 commits December 11, 2025 09:08

use FormatAdvice::default() where possible

e7615fd

fix: don't ignore errors

68aff12

When capturing clang tools output, any error encountered should cause fast-failure behavior. Also adjust run_main() tests so that erroneous output is more helpful..

ignore demo.hpp during LGTM tests about PR reviews

96dff79

This comment was marked as off-topic.

Sign in to view

2bndy5 merged commit fde6ce5 into main Dec 11, 2025
65 checks passed

2bndy5 deleted the fix-xml-parsing branch December 11, 2025 19:38

coderabbitai bot mentioned this pull request Dec 11, 2025

feat: use tokio::Command to capture suggestions #224

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: properly parse xml with no replacements#230

fix: properly parse xml with no replacements#230
2bndy5 merged 4 commits intomainfrom
fix-xml-parsing

2bndy5 commented Dec 11, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

2bndy5 commented Dec 11, 2025

Uh oh!

This comment was marked as off-topic.

2bndy5 commented Dec 11, 2025

Uh oh!

coderabbitai bot commented Dec 11, 2025

Uh oh!

codecov bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

Uh oh!

Conversation

2bndy5 commented Dec 11, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

This comment was marked as resolved.

Uh oh!

2bndy5 commented Dec 11, 2025

Uh oh!

This comment was marked as off-topic.

2bndy5 commented Dec 11, 2025

Uh oh!

coderabbitai bot commented Dec 11, 2025

Uh oh!

codecov bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

2bndy5 commented Dec 11, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 11, 2025 •

edited

Loading

codecov bot commented Dec 11, 2025 •

edited

Loading